fix: skip reorder when file has parse errors to prevent data loss#234
fix: skip reorder when file has parse errors to prevent data loss#234nojaf wants to merge 1 commit into
Conversation
When --reorder-code is used on a file with invalid syntax, tree-sitter wraps the affected function in an ERROR node. The reorder query uses (_) which silently excludes ERROR nodes, causing entire functions to be dropped from the output. Guard the reorder step with has_error() so files with parse errors are left untouched rather than losing declarations silently.
|
Thank you for taking the time to report and try to provide a fix. I don't think this is how this should be addressed, the parse error should not bubble up to the function definition. This is the sort of thing that should be addressed upstream and not hacked around here because this kind of code will hide errors and make us miss opportunities to consolidate the parser for the entire Godot ecosystem. Also, the parser already have the --safe flag for this sort of case, which will prevent the formatter from running until you've addressed the syntax error. Running the formatter without this option is something I'd mostly recommended to people who want to contribute to this project and help improve the quality of the tree-sitter parser (which in turn improves GDScript support in third party code editors) and of the formatter. |
2 participants
Please check if the PR fulfills these requirements:
Related issue (if applicable): #233
What kind of change does this PR introduce?
Bug fix.
Does this PR introduce a breaking change?
No.
New feature or change
What is the current behavior?
When running with
--reorder-codeon a file that contains invalid GDScript syntax, the formatter silently deletes entire function definitions. For example, a trailing.after a type annotation (var yozora: bool.) causes tree-sitter's error recovery to wrap the surrounding function in anERRORnode. The reorder query uses(_)as a wildcard, which excludesERRORnodes, so the affected function is never collected and simply disappears from the output.What is the new behavior?
Before running the reorder step, the formatter checks
tree.root_node().has_error(). If parse errors are present, reordering is skipped and the already-formatted code is returned as-is, with a warning printed to stderr. All functions are preserved.Other information
A regression test has been added in
tests/reorder_code/covering the exact scenario from the issue.Claude Code was used to create this PR but I did human verify the changes.